Skip to content

fix(media): re-enable React Compiler for the asset details form - #1841

Draft
pedrobonamin wants to merge 3 commits into
mainfrom
cursor/media-react-compiler-fix-1d31
Draft

fix(media): re-enable React Compiler for the asset details form#1841
pedrobonamin wants to merge 3 commits into
mainfrom
cursor/media-react-compiler-fix-1d31

Conversation

@pedrobonamin

@pedrobonamin pedrobonamin commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #1840, rebased on main: removes the temporary 'use no memo' compiler opt-out from Details and fixes the Save-button bug at its actual root cause, so the plugin ships fully compiled again.

Root cause

DialogAssetEdit calls reset(generateDefaultValues(...)) in an effect when the dialog mounts and again whenever the asset is updated elsewhere (mutation listener). react-hook-form's reset() empties its internal field registry (_fields = {}) and relies on the render-time register() calls re-running on the next render to re-register every field. React Compiler memoizes the registered-field JSX in Details keyed on the stable register reference and asset-derived values, so register() never re-runs after a reset. From then on, typing in filename / title / alt text / description updates the DOM (the inputs are uncontrolled) but react-hook-form ignores the events — isDirty never flips and Save stays disabled. Tags kept working because they use Controller, which doesn't depend on render-time register() re-invocation.

Fix

  • Pass {keepFieldsRef: true} to both reset() call sites (DialogAssetEdit, DialogTagEdit). This react-hook-form option keeps fields registered and applies the new values via setValue instead of wiping the registry.
  • Remove 'use no memo' from Details — with the reset fixed, the compiler-memoized form works correctly.
  • Convert the remaining inline const Footer = () => … components (and Header in DialogConfirm) to plain JSX, matching what fix(media): enable Save when editing asset string fields #1840 already did for DialogAssetEdit. An inline component gets a new function identity whenever captured values change, so React unmounted and remounted the whole footer subtree — including the Save button — on every form-state change. This also removes all the react/react-compiler lint suppressions that the pattern required.

Making the regression testable

Vitest previously ran against uncompiled source, so no test could catch compiler-specific regressions (the plugin ships compiled — reactCompiler: true in tsdown.config.ts). The plugin's Vitest config now applies the same @rolldown/plugin-babel + reactCompilerPreset combination that the build uses, so the whole suite — including the regression test added in #1840 — exercises compiler output.

Counterfactual check: removing 'use no memo' without keepFieldsRef fails 3 tests under the compiler ("enables Save after editing a string field (title)", "dispatches asset update when a field changes and the form is submitted", "persists a cleared Description as empty string so EXIF cannot refill it"); with keepFieldsRef all 205 pass. This also shows the useFormState / explicit onChange threading from #1840 were not the load-bearing parts of that workaround — only the compiler opt-out was.

Test plan

  • pnpm --filter sanity-plugin-media exec vitest run — 205 tests pass, now exercising compiler output with no 'use no memo'
  • Counterfactual: same suite fails 3 tests when keepFieldsRef is removed
  • pnpm format, pnpm lint, pnpm knip, pnpm build, pnpm test run (1281 tests) all pass
  • Manual verification in dev/test-studio (pnpm dev, compiler enabled via reactCompiler: {} in sanity.cli.ts), reproducing the reset-triggered failure mode in a real browser by patching the asset via the API while the dialog is open:
    • Without keepFieldsRef (compiler on, no 'use no memo'): after the externally-triggered form reset, typing in Title/Alt Text updates the inputs but Save stays disabled — bug reproduced.
    • With this branch: after the same external update and reset, typing re-enables Save and saving persists the edit.

Bug reproduction (broken build — Save stays disabled after the externally-triggered reset):

bug_repro_save_stays_disabled_without_fix.mp4

Fix demo (this branch — Save re-enables after the same reset and the edit saves):

fix_demo_save_enables_after_external_update.mp4

To show artifacts inline, enable in settings.

Open in Web Open in Cursor 

@changeset-bot

changeset-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1bd3dd6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sanity-plugin-media Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Aug 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
plugins-studio Ready Ready Preview Aug 5, 2026 2:58pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plugins-e2e-test-studio Ignored Ignored Aug 5, 2026 2:58pm

Request Review

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

✅ E2E Tests

🟢 26 passedview full reportview run

Studio: https://plugins-e2e-test-studio-27j1ckgza.sanity.dev

Datasets: pr-1841-chromium-31017836155, pr-1841-firefox-31017836155

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 32.49% 5911 / 18189
🔵 Statements 32.36% 6727 / 20784
🔵 Functions 27.4% 1393 / 5083
🔵 Branches 25.17% 3709 / 14734
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
plugins/sanity-plugin-media/src/components/DialogAssetEdit/Details.tsx 93.65% 94.89% 85.71% 95.12% 17, 19-20
plugins/sanity-plugin-media/src/components/DialogAssetEdit/index.tsx 83.22% 81.05% 72.41% 84.26% 34, 97, 156, 169-172, 179-184, 190, 198, 207, 217, 226-254, 260, 299, 316-318, 325-330, 390-391, 433, 500, 525, 528, 42-123, 122-123, 42-287
plugins/sanity-plugin-media/src/components/DialogConfirm/index.tsx 1.96% 0% 0% 2.94% 15-66
plugins/sanity-plugin-media/src/components/DialogFolderCreate/index.tsx 1.4% 0% 0% 2.56% 22-30
plugins/sanity-plugin-media/src/components/DialogTagCreate/index.tsx 100% 100% 100% 100%
plugins/sanity-plugin-media/src/components/DialogTagEdit/index.tsx 90.75% 83.63% 90% 92.53% 68, 87, 101-107, 115-117, 123, 135, 135, 163-164
Generated in workflow #8726 for commit 1bd3dd6 by the Vitest Coverage Report Action

…React Compiler

A plain reset() empties react-hook-form's internal field registry and
relies on register() re-running on the next render to re-register every
field. React Compiler memoizes the registered-field JSX (keyed on the
stable register reference), so after the reset that runs when the asset
edit dialog opens, string fields stayed unregistered: typing updated the
DOM but never marked the form dirty and Save stayed disabled. Passing
keepFieldsRef keeps fields registered while still applying the new values.

Also convert inline Footer/Header dialog components to plain JSX so the
footer subtree (including the Save button) is not remounted whenever
captured form state changes, and drop the react/react-compiler lint
suppressions that pattern required.
The published build compiles sanity-plugin-media with React Compiler
(reactCompiler: true in tsdown.config.ts), but tests ran against
uncompiled source, so compiler-specific regressions could not be caught.
Apply the same @rolldown/plugin-babel + reactCompilerPreset combination
in the plugin's Vitest config and add a regression test asserting the
Save button enables after editing the title field.
Remove the temporary 'use no memo' opt-out from Details. The root cause
is fixed by keepFieldsRef on reset(): fields stay registered even though
the compiler memoizes the register() field JSX, so string-field edits
keep marking the form dirty.
@cursor
cursor Bot force-pushed the cursor/media-react-compiler-fix-1d31 branch from b32601f to 1bd3dd6 Compare August 5, 2026 14:57
@cursor cursor Bot changed the title fix(media): keep the React Compiler enabled and fix the asset-details Save button fix(media): re-enable React Compiler for the asset details form Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants